Provide stub so apps using goleak can build unchanged with tinygo. #72
Conversation
Tinygo can build a large subset of go programs already. Unfortunately, tinygo currently does not quite support goleak. As a temporary measure, use the tinygo build tag to provide a stub for goleak. This will expand the world of go programs that run properly on tinygo, and remove one more obstacle keeping embedded system and wasm users from adopting go and goleak. This can be reverted if/when tinygo is enhanced to fully support goleak.
|
|
|
Just for my education
Do you know what exactly is missing in tinygo for goleak to run? Is that even on their roadmap? Either way, I'd probably defer to @abhinav for a stamp here. |
|
Tinygo would have to e.g. implement runtime.Stack(). Whether or not this is on their roadmap, gracefully falling back to a no-op on tinygo feels like the right thing to do until tinygo decides to support goleak. |
For desktop systems I can see it happening some day, but for WebAssembly and embedded systems it probably won't happen. Because |
|
Details aside, software that uses goleak is increasingly going to be built with both go and tinygo, may as well make it easy. |
There was a problem hiding this comment.
I'm not against supporting tinygo, but the current approach of only stubbing VerifyNone doesn't seem ideal -- what if someone is using Find? I think users of VerifyTestMain will get a compile error with this approach (the method isn't included for tinygo, but no alternative is available).
Trying to reproduce the issue, when goleak is used, a tinygo test fails with:
panic: runtime error: index out of range
It looks like the panic is coming from:
func Current() Stack {
return getStacks(false)[0]
}This is because getStacks is unable to parse a stack (the output of runtime.Stack is empty), so we get an empty list, but try to access index 0, causing the panic.
I'll propose another approach: when we get no stacks, return an error indicating that stack parsing failed, and have VerifyNone log that goleak checks are skipped as the stack couldn't be parsed. That way it's not a hack specific to tinygo, but instead more general handling that could also apply to future go releases (or other go compilers) where the stack format is not recognised by goleak.
|
I like the idea. The message should probably be suppressed or only shown once on tinygo, though, otherwise it could dominate logs. |
|
2 year old PR and still didn't get merged! 🤷🏻♂️ |
@AbdullahWins, this comment by @prashantv mentions why the PR as it is cannot be merged, and what it'll take for it to get merged. |
Thanks for the fast ⏩ response and also the clarification. |
Tinygo can build a large subset of go programs already.
Unfortunately, tinygo currently does not quite support goleak.
As a temporary measure, use the tinygo build tag to provide a stub
for goleak. This will expand the world of go programs that
run properly on tinygo, and remove one more obstacle keeping
embedded system and wasm users from adopting go and goleak.
This can be reverted if/when tinygo is enhanced to fully
support goleak.
Fixes #71